Unit Test Results

122 files  +  8  122 suites  +8   2m 3s ⏱️ +44s
205 tests +  3  205 ✔️ +  3  0 💤 ±0  0 ❌ ±0 
690 runs  +12  690 ✔️ +12  0 💤 ±0  0 ❌ ±0 

Results for commit f188eb6. ± Comparison against base commit 7c7822a.

org.matrix.android.sdk.api.pushrules.PushRuleActionsTest ‑ test_action_parsing
org.matrix.android.sdk.api.pushrules.PushRulesConditionTest ‑ test_displayName_condition
org.matrix.android.sdk.api.pushrules.PushRulesConditionTest ‑ test_eventmatch_cake_condition
org.matrix.android.sdk.api.pushrules.PushRulesConditionTest ‑ test_eventmatch_cakelie_condition
org.matrix.android.sdk.api.pushrules.PushRulesConditionTest ‑ test_eventmatch_path_condition
org.matrix.android.sdk.api.pushrules.PushRulesConditionTest ‑ test_eventmatch_type_condition
org.matrix.android.sdk.api.pushrules.PushRulesConditionTest ‑ test_eventmatch_words_only_condition
org.matrix.android.sdk.api.pushrules.PushRulesConditionTest ‑ test_notice_condition
org.matrix.android.sdk.api.pushrules.PushRulesConditionTest ‑ test_roommember_condition

im.vector.app.features.crypto.UISIDetectorTest ‑ If event decrypted during grace period should not trigger detection
im.vector.app.features.crypto.UISIDetectorTest ‑ trigger detection after grace period
im.vector.app.features.onboarding.ftueauth.FtueMissingRegistrationStagesComparatorTest ‑ when ordering stages, then prioritizes email
org.matrix.android.sdk.api.session.pushrules.PushRuleActionsTest ‑ test_action_parsing
org.matrix.android.sdk.api.session.pushrules.PushRulesConditionTest ‑ test_displayName_condition
org.matrix.android.sdk.api.session.pushrules.PushRulesConditionTest ‑ test_eventmatch_cake_condition
org.matrix.android.sdk.api.session.pushrules.PushRulesConditionTest ‑ test_eventmatch_cakelie_condition
org.matrix.android.sdk.api.session.pushrules.PushRulesConditionTest ‑ test_eventmatch_path_condition
org.matrix.android.sdk.api.session.pushrules.PushRulesConditionTest ‑ test_eventmatch_type_condition
org.matrix.android.sdk.api.session.pushrules.PushRulesConditionTest ‑ test_eventmatch_words_only_condition
…

♻️ This comment has been updated with latest results.

some things to look out for when making changes to this area

• spam when using a connected device like a smart watch/fitness band Notification keeps resending #1673
• double notifications when receiving fastlane updates followed by a sync Doubled notifications #4152

Hopefully the combination of all fixes in this area might have solved these without setAlertOnce() 🤞

for #1673 - tested locally on a mi band 5 and can confirm there's no regression

however #4152 returns, a way to test this locally is to apply a delay to https://github.com/vector-im/element-android/blob/develop/vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt#L174

coroutineScope.launch {
    delay(5000)
    session.requireBackgroundSync()
}

to avoid double notifications we'll need to further refine the notifications to dynamically enable alertOnce if the notification is new or an update

@ouchadam I had tested the #4152 by adding the "delay" and i didn't reproduce the bug.

@mnaturel I didn't reproduce your bug with an emulator.
I had tested with long and short messages, good and bad network.
Can you test it on device ?

@mnaturel I didn't reproduce your bug with an emulator. I had tested with long and short messages, good and bad network. Can you test it on device ?

After some tests on real device (Nokia 7.2, Android 11) I could not reproduce the double notification bug. I suspect it may happen sometimes on very high connectivity.

To clarify the double the notification, the notification UI is correct but two pings will be triggered for each message that goes through the fastlane flow (push from server -> fetch message directly -> notify message -> request sync -> sync finds new messages and notifies again) https://github.com/vector-im/element-android/blob/develop/vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt#L174

The double ping will only happen if the sync triggered by the push returns after the getEvent/fastlane , otherwise the event is ignored. The fastlane flow is only entered when on wifi and we're not currently viewing the room the message is part of https://github.com/vector-im/element-android/blob/develop/vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt#L187

Thanks @ouchadam for your explanation, it's more clear now.
Should we do a verification to activate or not the setOnlyAlertOnce ? to avoid the double ping.
Should we check if we get a notification coming from the sync ? In which case do we have to display notification coming from the sync ?
Should we only display the notifications coming from getEvent/fastlane ?
Is it possible to know if a notification is already shown ?
I don't know what is the best solution :/.

ultimately my gut feeling is that we'll need to dynamicly apply the setOnlyAlertOnce based on new content or updates/edits, I'm thinking we could check if the latest room message event is an update, and if so use setOnlyAlertOnce(true) to avoid the double ping

there's a few changes needed to enable this

• We need to confirm that notifying twice with setOnlyAlertOnce(false) setOnlyAlertOnce(true) only pings once

• Add state to know if an existing event has been replaced https://github.com/vector-im/element-android/blob/develop/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt#L115

• When creating the room notifications https://github.com/vector-im/element-android/blob/develop/vector/src/main/java/im/vector/app/features/notifications/RoomGroupMessageCreator.kt#L69 make use of the new state, could be something like setOnlyAlertOnce(events.last().isUpdate)

thanks for the update, can confirm it's working for me locally 💯